None of the below tests are async, we should mark them with async and have proper end to end async validation.

The method under test, get_response_headers(), is identical on both the sync and async pagers - both are a plain def returning self._response_headers.copy(). There is nothing to await inside it. So rewriting these unit tests as async def would add an event loop and ceremony that will still do the same in m emory copy.
this test pins one contract: if a client ask for headers before any page has been fetched, they get back an empty case-insensitive dict — not None, not an exception.

we already have end to end validation tests
test_query_response_headers_async.py uses IsolatedAsyncioTestCase, creates a real container, runs query_items, does real async for item in query_iterable (and by_page() iteration), and only then asserts on get_response_headers() - including multi-page, empty-result, concurrent, and per-page-overwrite cases.
test_cosmos_responses_async.py does the same shape for async point operations - await upsert_item, create_item, read_item, batch — then checks the returned headers

Please let me know if I am missed your point.

🔴 Correctness — Strict-mode decode tests are environment-dependent

The test_sync_inference_2xx_with_invalid_utf8_raises_decode_error and its async counterpart expect DecodeError on malformed UTF-8, but they don't isolate AZURE_COSMOS_CHARSET_DECODER_ERROR_ACTION_ON_MALFORMED_INPUT. The setUp/tearDown only save/restore the inference endpoint env var, not the charset decoder env var.

If that env var is set to REPLACE or IGNORE in the process (e.g., by CI configuration, another test, or a developer's shell), decode_response_body_for_status() at _response_decoding.py:49-52 will decode permissively instead of raising, and assertRaises(DecodeError) will fail with DecodeError not raised — or worse, silently stop testing the strict-mode contract.

The REPLACE/IGNORE tests in the same file correctly save/restore the env var manually — but these strict-mode tests assume it's absent. The same issue exists in test_request_response_decoding.py:163.

Suggestion: Add env-var isolation to setUp/tearDown:

def setUp(self):
    self._saved_endpoint = os.environ.get(_INFERENCE_ENDPOINT_ENV_VAR)
    os.environ[_INFERENCE_ENDPOINT_ENV_VAR] = "https://example.com"
    self._saved_malformed = os.environ.get(_MALFORMED_INPUT_ENV_VAR)
    os.environ.pop(_MALFORMED_INPUT_ENV_VAR, None)  # ensure strict mode

def tearDown(self):

    # ... existing endpoint restore ...
    if self._saved_malformed is not None:
        os.environ[_MALFORMED_INPUT_ENV_VAR] = self._saved_malformed
    else:
        os.environ.pop(_MALFORMED_INPUT_ENV_VAR, None)

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🔴 Correctness — Same env-var dependency as test_semantic_reranker_unit.py

This test expects DecodeError on a 200 response with invalid UTF-8 in default strict mode, but it doesn't clear AZURE_COSMOS_CHARSET_DECODER_ERROR_ACTION_ON_MALFORMED_INPUT. If that env var is set to REPLACE or IGNORE in the process, decode_response_body_for_status() decodes permissively and the assertRaises(DecodeError) assertion fails — or stops testing the strict-mode contract silently if the test runner swallows the failure.

The test_response_decoding.py file already solved this by adding an env-isolating base class. This file should do the same, or at minimum os.environ.pop("AZURE_COSMOS_CHARSET_DECODER_ERROR_ACTION_ON_MALFORMED_INPUT", None) in a setup method.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Recommendation — Aggregate timeout assertion allows silent regression on page fetches 2+

The None-filtering here weakens the assertion compared to the non-aggregate sibling tests. Consider this scenario:

1. A regression causes only the first /docs call (the query-plan POST) to carry read_timeout.
2. doc_timeouts = [22, None, None, None] — subsequent page fetches lost the timeout.
3. non_none = [22] → len(non_none) > 0 ✅ → all(rt == 22 for rt in non_none) ✅ — both assertions pass despite the bug.

The non-aggregate paging tests (test_read_timeout_propagates_through_by_page_paging) assert all(rt == timeout for rt in doc_timeouts) with no None filtering, which would catch this same regression.

The comment says the relaxation is for "the query-plan POST against /docs/" — but the query-plan POST and the page fetches can be distinguished by HTTP method (POST vs GET) or by a tighter URL filter.

Suggestion: Assert that at most one /docs call (the query plan) has None, and all the rest carry the expected value:

none_count = sum(1 for rt in doc_timeouts if rt is None)
self.assertLessEqual(none_count, 1, "More than one /docs call dropped read_timeout")
non_none = [rt for rt in doc_timeouts if rt is not None]
self.assertTrue(all(rt == client_timeout for rt in non_none))

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Recommendation — Sync/async parity gap: enable_cross_partition_query=True omitted in all async aggregate tests

Every sync aggregate test in test_query.py explicitly passes enable_cross_partition_query=True, but the async counterparts in this file omit it entirely (zero occurrences across all new async tests). The same issue was raised and fixed for test_read_timeout_propagation_async.py — this file needs the same treatment.

While the async container auto-enables cross-partition queries when no partition_key is given, the explicit flag exercises a different code path (feed_options["enableCrossPartitionQuery"] = True set by the caller vs. auto-detected by the SDK). If a regression only affects the explicit-flag path in the async client, these tests would not catch it.

Suggestion: Add enable_cross_partition_query=True to all async cross-partition aggregate test queries, matching the sync counterparts.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Recommendation — Identity check pins a private import alias, not behavior

assert _async_conn.base._merge_query_results is _base._merge_query_results

The attribute base only exists because _cosmos_client_connection_async.py:50 uses from .. import _base as base. If someone refactors this to from .. import _base (dropping the as base alias), this test raises AttributeError: module has no attribute 'base' — not because a behavior changed, but because an internal symbol name changed.

This is a false-positive risk: the test fails when the code is correct, and the error message won't point at a real bug.

Suggestion: Test the behavioral contract instead — e.g., verify that calling _merge_query_results from the async code path produces the same output as from the sync path. Or at minimum, access the module's _base attribute directly (_async_conn._base._merge_query_results) which matches the actual import name and is less fragile.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Recommendation — Sub-picosecond timeout is unreliable across environments

read_timeout=0.000000000001 is 1 picosecond. OS timer resolution on Linux is typically 1 µs–1 ms; Windows is ~15.6 ms. The kernel will round this up to its minimum granularity. On fast local networks (e.g., emulator on localhost), the HTTP round-trip can complete before the timer resolution kicks in, meaning no timeout fires and assertRaises raises AssertionError: CosmosClientTimeoutError not raised.

This pattern is inherently flaky — it depends on the request being slower than the OS timer minimum, which varies by machine and load.

Suggestion: Use a mock transport that injects a configurable delay (like the _CaptureTransport pattern elsewhere), or set read_timeout to something slightly larger but still practically guaranteed to expire (e.g., 0.0001 = 100 µs against a remote endpoint). Alternatively, if the intent is just to prove the kwarg reaches the wire, the _capture_pipeline_read_timeouts pattern already used above is more deterministic.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Recommendation — Async test hardcodes self.masterKey while sync uses self.credential

All async client-level timeout tests create the client with self.masterKey:

async with CosmosClient(self.host, self.masterKey, read_timeout=...) as ct_client:

But the sync counterparts use self.credential:

with cosmos_client.CosmosClient(self.host, self.credential, read_timeout=...) as ct_client:

self.credential can be either a master key or an AAD credential depending on the CI lane configuration. By hardcoding self.masterKey, the async tests bypass AAD token-refresh paths entirely. In AAD CI lanes, the sync tests exercise AAD + read_timeout interaction (where the timeout could fire during token acquisition), but the async tests never do.

Suggestion: Use self.credential (or test_config.TestConfig.create_data_client_async(read_timeout=...)) for consistency with the sync tests.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Recommendation — Module-scope import defeats the packaging test

This module-level import means if the aio extras are broken (e.g., aiohttp is missing), the entire test module fails during collection with an ImportError before test_aio_extras_declared_in_distribution_metadata can run and report a targeted failure.

The test's purpose is to validate the aio extra is declared correctly in package metadata. If the import is broken, the most useful outcome is the metadata test failing with a clear message like "aio extra not found in distribution metadata" — not a raw ImportError during collection that doesn't explain the root cause.

Suggestion: Move the import inside test_azure_cosmos_aio_module_imports():

def test_azure_cosmos_aio_module_imports(self):
    """Importing from azure.cosmos.aio should work when the aio extra is installed."""
    from azure.cosmos.aio import CosmosClient  # noqa: F401
    self.assertIsNotNone(CosmosClient)

This way the metadata test can run independently and report whether the package metadata is correct, even when the import itself is broken.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟢 Suggestion — Guard tracemalloc.start()/stop() against external tracing tools

The tracemalloc.stop() is now properly in a finally block (good), but start() is still called unconditionally. If an external tool (e.g., pytest-memray, a memory profiler, or another test) already has tracing active, this test's stop() will terminate their tracing session.

Python docs: tracemalloc.stop() stops all tracing regardless of who started it. start() when already active only updates nframe.

Suggestion:

was_tracing = tracemalloc.is_tracing()
if not was_tracing:
    tracemalloc.start()
try:
    snapshot_before = tracemalloc.take_snapshot()

    # ... iteration ...
finally:
    if not was_tracing:
        tracemalloc.stop()

Same pattern should apply to the async mirror.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟢 Suggestion — Mock signature lacks **kwargs — fragile against future callers

async def _always_multi_overlap(_rid, feed_ranges, _opts):

The real get_overlapping_ranges signature accepts **kwargs. If a future change adds a keyword argument to the call site (e.g., for tracing or retry context), this mock raises TypeError: _always_multi_overlap() got an unexpected keyword argument — a test failure caused by a correct production change.

The same pattern appears in the sync sibling test_query_feed_range_multipartition.py.

Suggestion: Add **_kwargs to both mocks:

async def _always_multi_overlap(_rid, feed_ranges, _opts, **_kwargs):

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

💬 Observation — Docstring trim removes the rationale for _clean_location_list's existence

The previous docstring explained why this function exists: _normalize_region_name(None) returns "", and unfiltered None/empty entries in exclusion lists would collide with "" in the by-endpoint lookup map, causing every endpoint that isn't found in the map to silently match the excluded entry. The new docstring says "so blank inputs cannot accidentally match real endpoints" — which is correct but doesn't explain the mechanism.

A future maintainer looking at this helper could reasonably conclude it's just defensive overcaution and remove it, re-introducing the silent exclusion bug where None entries in excluded_locations cause every non-mapped endpoint to be excluded.

Consider keeping a one-line hint about the _normalize_region_name(None) → "" collision:

"""Return the list with None, empty, and whitespace-only entries removed.

Used at every location-list entry point so blank inputs cannot accidentally
match real endpoints: _normalize_region_name(None) returns "", which would
collide with any missing endpoint lookup during later comparisons.
"""

Not blocking — the behavior is correct and the code is clear on its own.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟢 Suggestion — Tests exercise per-response decode isolation, not actual pager state

The module docstring says "paging through one response after another keeps working" and the test class names include "Paged", but _drive_pages() calls _synchronized_request._Request() (a single-response decode function) once per body with completely fresh mock args each time. No CosmosItemPaged, ItemPaged, or continuation-token state carries between calls.

This is still useful — it validates that the codec doesn't leak corrupt state across repeated _Request() invocations (the bad→good→bad 3-page sequence is particularly good). But a regression in pager-level continuation handling, iterator state, or response_headers lifecycle across pages would not be caught by these tests.

The test name could better reflect what it actually proves. Something like test_response_decoding_multi_call.py or adjusting the module docstring to say "calling the request function multiple times in sequence" rather than "paging" would prevent confusion.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.